[Belt] Tweak internal representation output #2536
Conversation
|
cc @keirah =) |
|
should not height be least important? in general, the second commit would be much easier to review and merged, it would be good to split PRs into small commits |
Second diff of rescript-lang#2536, pulled out
|
K, I've pulled the second part out to #2539. Height is indeed less important, but having left and right at the end formats better in node repl and chrome console, so I sandwiched it in the middle. Top and bottom are important. Plus, I don't think we should mingle not being important with being cryptic (the other names are well chosen). |
|
Rebased. |
|
@bobzhang the other PR is accepted, so this one's now just about the height and field position change. Before: Note that in either vertical or horizontal reading, the value and subtree are now easier to find. |
This puts the most important stuff in prominent position. Before:
```js
{ left: { left: null, key: 1, right: null, h: 1 },
key: 2,
right:
{ left: null,
key: 3,
right: { left: null, key: 5, right: null, h: 1 },
h: 2 },
h: 3 }
```
After:
```js
{ value: 2,
height: 3,
left: { value: 1, height: 1, left: null, right: null },
right:
{ value: 3,
height: 2,
left: null,
right: { value: 5, height: 1, left: null, right: null } } }
```
Friendlier in browser consoles too. Explicitly name "h" to "height" for extra education.
|
Rebased again |
|
see my comments above, should height be moved to the last position(maybe called |
|
I definitely agree that left/right should be grouped together (not split by key/value). Less opinionated about whether it happens at the top or the bottom. I think @chenglou's argument for the end is similar to the |
|
@bobzhang height should not be at the last position; in the screenshot it's hard to see where value, left and right are when height is not stuffed in the middle |
|
Left/right at the top means you get a chunk of trailing closing heights when opening deep values in the inspector |
|
I have no strong opinions since this would not affect user side any way, feel free to merge it after CI |
|
Cool! Thanks |
3 participants
Addresses a first wave of beta feedback.
This puts the most important stuff in prominent position. Before:
After:
Friendlier in browser consoles too. Explicitly name "h" to "height" for extra education.